Conversation
ben-grande
force-pushed
the
st-test
branch
2 times, most recently
from
77fae4e to
4318162
Compare
- Check output when no argument is provided; - Check if stdin is ignored by default; - Check if stdin is ignored if file is provided; - Check how many arguments a utility accepts; - Check how word splitting works on multiple arguments; - Check if newline is present; and - How it treats the argument '-'.
ben-grande
force-pushed
the
st-test
branch
7 times, most recently
from
0cc3cbe to
2e92192
Compare
ArrayBolt3
suggested changes
May 11, 2025
Contributor
ArrayBolt3
left a comment
ArrayBolt3
left a comment
There was a problem hiding this comment.
Tests pass both as-is and with the couple of functional changes suggested.
Comment on lines
34
to
51
| i = 0 | ||
| self.tmpdir = tempfile.mkdtemp() | ||
| while i < 6: | ||
| self.tmpfiles_list.append(os.path.join(self.tmpdir, str(i))) | ||
| with open(self.tmpfiles_list[i], "w", encoding="utf-8") as file: | ||
| if i == 0: | ||
| file.write("") | ||
| elif i == 1: | ||
| file.write("".join(contents)) | ||
| elif i == 2: | ||
| file.write("".join(contents) + "\n") | ||
| elif i == 3: | ||
| file.write(self.text_dirty) | ||
| elif i in [4, 5]: | ||
| pass | ||
| file.flush() | ||
| file.close() | ||
| i += 1 |
Contributor
There was a problem hiding this comment.
Suggested change
| i = 0 | |
| self.tmpdir = tempfile.mkdtemp() | |
| while i < 6: | |
| self.tmpfiles_list.append(os.path.join(self.tmpdir, str(i))) | |
| with open(self.tmpfiles_list[i], "w", encoding="utf-8") as file: | |
| if i == 0: | |
| file.write("") | |
| elif i == 1: | |
| file.write("".join(contents)) | |
| elif i == 2: | |
| file.write("".join(contents) + "\n") | |
| elif i == 3: | |
| file.write(self.text_dirty) | |
| elif i in [4, 5]: | |
| pass | |
| file.flush() | |
| file.close() | |
| i += 1 | |
| self.tmpdir = tempfile.mkdtemp() | |
| for i in range(0, 6): | |
| self.tmpfiles_list.append(os.path.join(self.tmpdir, str(i))) | |
| with open(self.tmpfiles_list[i], "w", encoding="utf-8") as file: | |
| if i == 0: | |
| file.write("") | |
| elif i == 1: | |
| file.write("".join(contents)) | |
| elif i == 2: | |
| file.write("".join(contents) + "\n") | |
| elif i == 3: | |
| file.write(self.text_dirty) | |
| elif i in [4, 5]: | |
| pass | |
| file.flush() | |
| file.close() |
Contributor
Author
There was a problem hiding this comment.
Github browser diff is bad on this case. Is the change while to for and removal of the individual i var assignments?
Contributor
There was a problem hiding this comment.
Is the change while to for and removal of the individual i var assignments?
Yes.
Github browser diff is bad on this case.
Yeah... not sure why it didn't render this very well this time.
| shutil.rmtree(self.tmpdir) | ||
|
|
||
| def _del_module(self) -> None: | ||
| for module in ["stdisplay." + self.module]: # type: ignore # pylint: disable=no-member |
Contributor
There was a problem hiding this comment.
It might be useful to document that self.module is going to be defined in the individual tests themselves.
Comment on lines
95
to
101
| def _get_file(self, file: str) -> str: | ||
| """ | ||
| Helper function get contents of a file. | ||
| """ | ||
| with open(file, mode="r", encoding="utf-8") as fileobj: | ||
| text = fileobj.read() | ||
| return text |
Contributor
There was a problem hiding this comment.
Could Path.read_text() be used instead of a custom helper function for this?
Contributor
Author
There was a problem hiding this comment.
Makes sense. Done.
| import stdisplay.tests | ||
|
|
||
|
|
||
| class TestSTCat(stdisplay.tests.TestSTBase): |
Contributor
There was a problem hiding this comment.
Suggested change
| class TestSTCat(stdisplay.tests.TestSTBase): | |
| class TestSTCatn(stdisplay.tests.TestSTBase): |
| class TestSTPrint(stdisplay.tests.TestSTBase): | ||
| """ | ||
| Test stprint | ||
| Test stecho. |
Contributor
There was a problem hiding this comment.
Suggested change
| Test stecho. | |
| Test stprint. |
| "", | ||
| self._get_file(file=self.tmpfiles["fill"]), | ||
| ) | ||
| # Empty stdin when writing to file and file sanitization. |
Contributor
There was a problem hiding this comment.
Suggested change
| # Empty stdin when writing to file and file sanitization. | |
| # Empty stdout when writing to file and file sanitization. |
| self.text_dirty_sanitized, | ||
| self._get_file(file=self.tmpfiles["fill"]), | ||
| ) | ||
| # Empty stdin when writing to multiple files and its sanitization. |
Contributor
There was a problem hiding this comment.
Suggested change
| # Empty stdin when writing to multiple files and its sanitization. | |
| # Empty stdout when writing to multiple files and its sanitization. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
Add tests for safe terminal utilities.
Mandatory Checklist
Terms of Service, Privacy Policy, Cookie Policy, E-Sign Consent, DMCA, Imprint
Optional Checklist
The following items are optional but might be requested in certain cases.
Fixes: